refactor(semver): clean up parseRange, add missing tests
#6362
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6362 +/- ##
==========================================
+ Coverage 96.23% 96.30% +0.06%
==========================================
Files 556 556
Lines 42065 42063 -2
Branches 6371 6371
==========================================
+ Hits 40481 40507 +26
+ Misses 1544 1517 -27
+ Partials 40 39 -1 ☔ View full report in Codecov by Sentry. |
| prerelease: [], | ||
| build: [], | ||
| }]]; | ||
| assertEquals(lessThanRange(version, range), true); |
There was a problem hiding this comment.
This result feels wrong to me. I'd suggest skipping this test case for now
There was a problem hiding this comment.
I think we should not skip test cases if they yield unexpected results but fix them instead. What would the proper result be?
There was a problem hiding this comment.
What would the proper result be?
The range != 1.0.0 should include all versions except 1.0.0. So I think the expected result here is false.
There was a problem hiding this comment.
Would it make sense to create a PR to remove the != straight away as a bug fix before landing this PR? This probably solves lots of headaches with intermediate patches.
There was a problem hiding this comment.
That will be a breaking change and we currently don't have a plan for it.
There was a problem hiding this comment.
I think this bug could be fixed by modifying the != case to return cmp === 0; instead of return false; in greaterThanComparator() and lessThanComparator(). Then it will return the expected false.
| }); | ||
|
|
||
| Deno.test({ | ||
| name: "compare() handles undefined in prerelease", |
There was a problem hiding this comment.
This test case looks comparing invalid semvers. I don't think this is an intentional behavior, but an undefined behavior. I'd suggest we should remove this test case.
There was a problem hiding this comment.
If this is undefined behavior we probably should remove if (a === undefined && b === undefined) return 0; from compareIdentifier() as this is specifically handled there.
kt3k
changed the title
parseRange, add missing tests
No description provided.